Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GODRIVER-2962 Remove setters from Command. #1364

Closed
wants to merge 1 commit into from
Closed

GODRIVER-2962 Remove setters from Command. #1364

wants to merge 1 commit into from
+151 −219

Conversation

matthewdale
Copy link
Collaborator

GODRIVER-2962

Summary

Remove setters from Command.

Background & Motivation

The field setters on the operation types in the x/mongo/driver/operation package provide no usability improvement but create a maintenance burden whenever we have to update the operations. Remove the field setters and use struct literals instead.

Split the change into multiple PRs to make reviewing them easier.

@matthewdale matthewdale requested a review from a team as a code owner August 28, 2023 23:39
@matthewdale matthewdale requested review from prestonvasquez and removed request for a team August 28, 2023 23:39
@matthewdale matthewdale temporarily deployed to api-report August 28, 2023 23:39 — with GitHub Actions Inactive
@github-actions
Copy link

API Change Report

./x/mongo/driver/operation

incompatible changes

(*Command).ClusterClock: removed
(*Command).CommandMonitor: removed
(*Command).Crypt: removed
(*Command).Database: removed
(*Command).Deployment: removed
(*Command).Logger: removed
(*Command).ReadConcern: removed
(*Command).ReadPreference: removed
(*Command).ServerAPI: removed
(*Command).ServerSelector: removed
(*Command).Session: removed
(*Command).Timeout: removed
NewCommand: removed
NewCursorCommand: removed

compatible changes

Command.Clock: added
Command.Command: added
Command.CreateCursor: added
Command.Crypt: added
Command.CursorOpts: added
Command.Database: added
Command.Deployment: added
Command.Logger: added
Command.Monitor: added
Command.ReadPreference: added
Command.Selector: added
Command.ServerAPI: added
Command.Session: added
Command.Timeout: added

prestonvasquez
prestonvasquez previously approved these changes Aug 31, 2023
View reviewed changes
@prestonvasquez prestonvasquez changed the base branch from master to v1 September 13, 2023 18:05
@matthewdale matthewdale changed the base branch from v1 to master September 20, 2023 04:36
@matthewdale matthewdale dismissed prestonvasquez’s stale review September 20, 2023 04:36

The base branch was changed.

@matthewdale matthewdale force-pushed the godriver2962-remove-op-setters-2 branch from 934fba2 to ff57da9 Compare September 20, 2023 04:37
prestonvasquez
prestonvasquez approved these changes Sep 22, 2023
View reviewed changes
Copy link
Collaborator

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a v2 change? GODRIVER-2962 is not part of the 2.0 epic, but this PR is pointing at master.

@matthewdale
Copy link
Collaborator Author

matthewdale commented Oct 11, 2023 •
edited
Loading

@prestonvasquez no, this is not a strictly v2 change. I switched the branch to minimize the additional work required to sync changes from v1 to master because it isn't required in v1.

@matthewdale
Copy link
Collaborator Author

Closing this for now, will follow up later when GODRIVER-2962 is scheduled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prestonvasquez prestonvasquez prestonvasquez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matthewdale @prestonvasquez